✨ Password Policy for LDAP Directories #105
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
==========================================
- Coverage 97.55% 97.33% -0.22%
==========================================
Files 4 4
Lines 245 300 +55
Branches 67 106 +39
==========================================
+ Hits 239 292 +53
- Misses 6 8 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds password policy support to the OmniAuth LDAP strategy by implementing the LDAP Password Policy request/response control (OID 1.3.6.1.4.1.42.2.27.8.5.1). This allows applications to receive detailed information about password-related authentication failures (e.g., expired passwords, account locked, grace logins remaining).
Key Changes:
- Added
password_policyconfiguration option to enable password policy control requests - Modified
bind_asto attach password policy controls to bind requests when enabled - Exposed
last_operation_resultandlast_password_policy_responseaccessors on the adaptor - Added
attach_password_policy_envhelper to expose policy information via Rack env
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/omniauth-ldap/adaptor.rb | Added password policy control request logic to bind_as, control capture via capture_password_policy, and exposed accessors for operation result and policy response |
| lib/omniauth/strategies/ldap.rb | Added attach_password_policy_env and extract_password_policy helpers to expose password policy information via Rack env on authentication success/failure |
| sig/omniauth/ldap/adaptor.rbs | Updated type signatures: renamed constant from METHOD to ENCRYPTION_METHOD, added password policy accessors and helper method signatures, added missing instance variable declarations |
| sig/rbs/net-ldap.rbs | Added type signatures for get_operation_result, LDAP control classes, and PDU |
| sig/rbs/net-ntlm.rbs | Added target_name= writer signature and removed trailing blank line |
| sig/omniauth/strategies/ldap.rbs | Added method signatures for uid, info, and extra |
| sig/omniauth-ldap.rbs | Added OmniAuth::LDAP module declaration |
| spec/omniauth/strategies/ldap_spec.rb | Added comprehensive tests for password policy functionality and POST redirect behavior |
| spec/omniauth-ldap/adaptor_spec.rb | Added tests for password policy control attachment to binds and control capture |
| README.md | Added documentation and example for the password_policy option |
| CHANGELOG.md | Documented the new password policy feature |
Comments suppressed due to low confidence (2)
sig/omniauth/ldap/adaptor.rbs:1
- The instance variable
@bind_methodis set inlib/omniauth-ldap/adaptor.rbline 92 and used in lines 98, 100, and 123, but is not declared in the RBS type signature file. Add@bind_method: Symbol?to the instance variable declarations section (after line 69) to maintain type signature completeness.
module OmniAuth
sig/omniauth/ldap/adaptor.rbs:1
- Several instance variables from
VALID_ADAPTER_CONFIGURATION_KEYSare used in the adaptor code but are not declared in the RBS file. Based on the code inlib/omniauth-ldap/adaptor.rblines 82-84 and 171, add missing instance variable declarations for@encryption,@method,@hosts,@host, and@portto maintain consistency with the runtime behavior.
module OmniAuth
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pboling
force-pushed
the
feat/draft-behera-ldap-password-policy-11
branch
from
4adbd81 to
75864cd
Compare
Minimum allowed line rate is |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Password Policy for LDAP Directories
Implements and closes #44